fix(index): safely seed nested worktree indexes - #178

cmd/seed_test.go (1)

32-66: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Assert the status messages for the success paths.

The table only checks statuses when the seed stub fails. The branches in seedFromDonorIfNew at lines 82-90 of cmd/seed.go stay untested: the seeded case, the "Index was seeded by another process." case, and the "Sibling index could not be reused" case. A wantStatuses []string field on the table covers all of them.

♻️ Proposed test extension

 	tests := []struct {
 		name     string
 		setupDB  bool // pre-create the destination DB
 		donor    string
 		seedErr  error // error returned by the seed stub
 		wantFind bool  // donor discovery should run
 		wantSeed bool  // seed should run
+		wantStatuses []string
 	}{

 		{
 			name:     "seeds when DB missing and donor found",
 			donor:    "/donor.db",
 			wantFind: true,
 			wantSeed: true,
+			wantStatuses: []string{
+				"Seeding index from sibling worktree...",
+				"Seeded index from sibling worktree.",
+			},
 		},

 			} else if warning != "" {
 				t.Errorf("unexpected warning: %q", warning)
 			}
+			if tt.wantStatuses != nil && !slices.Equal(statuses, tt.wantStatuses) {
+				t.Errorf("statuses = %v, want %v", statuses, tt.wantStatuses)
+			}

Also applies to: 114-123

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@cmd/seed_test.go` around lines 32 - 66, Extend the seedFromDonorIfNew test
table with a wantStatuses []string field and assert the collected statuses for
every case, including successful seeding, an index seeded by another process,
and donor-reuse failure. Populate expected status messages for each branch while
preserving the existing seed and donor-discovery assertions.

internal/indexlock/lock.go (1)

57-63: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Return a sentinel error instead of an inline string.

errors.New("lock not acquired") allocates a new, uncomparable error on every call. Callers cannot detect this condition. Declare a package-level sentinel so callers can use errors.Is.

The coding guidelines require proper error types instead of generic error strings.

♻️ Proposed refactor

+// ErrNotAcquired reports that the lock could not be taken.
+var ErrNotAcquired = errors.New("index lock not acquired")
+
 // Acquire waits for an exclusive lock on lockPath or for ctx to be cancelled.

 		if err := ctx.Err(); err != nil {
 			return nil, err
 		}
-		return nil, errors.New("lock not acquired")
+		return nil, ErrNotAcquired

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@internal/indexlock/lock.go` around lines 57 - 63, Replace the inline
errors.New("lock not acquired") return in the lock acquisition flow with a
package-level sentinel error, and declare that sentinel in internal/indexlock.
Return the sentinel unchanged so callers can reliably detect the condition with
errors.Is, while preserving the existing ctx.Err() handling.

Source: Coding guidelines

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes